Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes #925: Invalid E.164 format for pt_PT PhoneNumber Provider #926

Open
wants to merge 3 commits into
base: 2.0
Choose a base branch
from
Open

Fixes #925: Invalid E.164 format for pt_PT PhoneNumber Provider #926

wants to merge 3 commits into from

Conversation

Head0nF1re
Copy link

@Head0nF1re Head0nF1re commented Dec 13, 2024 •
edited
Loading

Summary

  • Fix bug where e164PhoneNumber doesn't return pt_PT phone numbers when using the specific pt_PT provider. Override $e164Formats in order to provide locale specific phone numbers (mobile and landline) in E.164 format.

  • Add constants / static properties for common formatting codes (country, area and mobile service).

  • Add methods e164MobileNumber and e164LandlineNumber to provide mobile-only and landline-only phone numbers, respectively.

  • Add tests.

What is the reason for this PR?


Note: I wanted to use the same "formatting rules" for $mobileNumberPrefixes but I think there is some issue with the {{ }} substitution since the mobileNumber method is static and there's not static parse method.

Author's checklist

Review checklist

  • All checks have passed
  • Changes are added to the CHANGELOG.md
  • Changes are approved by maintainer

@Head0nF1re
Fixes FakerPHP#925
2570540 
- Override $e164Formats in order to provide locale specific phone
  numbers (mobile and landline) in E.164 format.

- Add constants / static properties for common formatting codes (country,
  area and mobile service).

- Add methods 'e164MobileNumber' and 'e164LandlineNumber' to provide
  mobile-only and landline-only phone numbers, respectively.
@Head0nF1re Head0nF1re changed the title Fixes FakerPHP/Faker#925 Fixes #925 Dec 13, 2024
@Head0nF1re Head0nF1re changed the title Fixes #925 Fixes #925: Invalid E.164 format for pt_PT PhoneNumber Provider Dec 13, 2024
@Head0nF1re Head0nF1re mentioned this pull request Dec 13, 2024
Closed
@Head0nF1re
Merge AREA_OR_MOBILE_SERVICE_CODE
3591ca3 
- Add consts in order to be able to merge AREA_CODE and
  MOBILE_SERVICE_CODE.
@stale Stale
Copy link

stale bot commented Jan 31, 2025

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 1 week if no further activity occurs. Thank you for your contributions.

@Head0nF1re
Add Regex test cases for pt_PT PhoneNumber FakerPHP#925
59cbcb5 
- Add tests for e164PhoneNumber, e164MobileNumber and
  e164LandlineNumber.

- Simplify regex for phoneNumber.

Note:
- Regex for phoneNumber was duplicating some information. It also
  validated for landline numbers starting at 20 even tho it wasn't
  present in the provider $formats.

- Regex for country code, mobile number and landline number could be
  placed in a const variable. The problem is: there's some issues with
  interpolating self/static variables and concatenation would probably
  be hard to read.
@stale stale bot removed the lifecycle/stale label Feb 8, 2025
@Head0nF1re Head0nF1re marked this pull request as ready for review February 8, 2025 18:54
@Head0nF1re
Copy link
Author

Head0nF1re commented Feb 8, 2025 •
edited
Loading

In #925, @pimjansen closed the issue saying:

This is as intended since the method is not implemented for the locale itself

As I explained in the reply to that, there were other PR's for different providers that fixed the same issue and it doesn't make much sense for a locale specific provider to not return locale specific data.

This is a simple PR that fixes it:

  • Add the $e164Formats to fix the issue of not providing pt_PT e164 phone numbers.
  • Add e164MobileNumber and e164LandlineNumber, a simple addition that allows for the retrieval of only mobile or landline.

Let me know if the new methods (e164MobileNumber and e164LandlineNumber) should not be introduced in this PR, if they can I will add the DocBlocks. I still think that's useful but I understand.

@pimjansen
Copy link

Can we do this PR on 1.x? For 2.x this code will be dropped and probably ported to a new repository

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
needs work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Invalid E.164 format for pt_PT PhoneNumber Provider
2 participants
@Head0nF1re @pimjansen